Skip to content

Conversation

@bcully
Copy link
Contributor

@bcully bcully commented Sep 22, 2025

A small protected method that passes through the provided reader by default.

@bcully bcully requested a review from lkts September 22, 2025 18:22
@bcully bcully added >non-issue :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v9.2.0 labels Sep 22, 2025
*/
@Override
public Bits getLiveDocs() {
ensureUnownedDocumentsPresent();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use more generic naming now that the logic is generic (multiple places)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, thanks for noting that.

@bcully bcully marked this pull request as ready for review September 22, 2025 22:05
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Sep 22, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

@bcully
Copy link
Contributor Author

bcully commented Sep 24, 2025

@jimczi @henningandersen this is an alternative to #134252 (still 90% @lkts code though) to address concerns you had there about
a) opening the door to an arbitrary number of reader wrappers - this adds a hook for only a single new wrapper.
b) the reader wrapper interface being powerful/dangerous - the new hook is reduced in power to only being able to filter out documents that match a provided query.
c) defined order between security and this new filter - here it's explicit, the new filter is the inner one and security outer.

Narrowing the interface to take a query does make the PR larger though. I'd be interested in your thoughs.

This is a more limited version of the existing ReaderWrapper
that only accepts a query of documents to filter from the
reader. It is meant to support temporarily filtering out
documents that no longer belong to a shard after it has
been split, until the shard is cleaned.

The decision about whether to install the filter is made
when the index module is created. To reduce overhead in
the common case, the query-supplying function may return
null in which case the underlying reader is returned
directly.

Co-authored-by: Oleksandr Kolomiiets <[email protected]>
@bcully bcully force-pushed the reshard-search-filter branch from c1bd812 to 2891b3f Compare September 24, 2025 18:08
@bcully
Copy link
Contributor Author

bcully commented Sep 24, 2025

I rebased this to add @lkts as co-author to the commit.

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Sep 24, 2025
@bcully
Copy link
Contributor Author

bcully commented Sep 29, 2025

@henningandersen @jimczi gentle reminder

bcully added 2 commits October 1, 2025 18:00
Adds a protected hook in Engine to allow wrapping the
DirectoryReader supplied to a Searcher.
@bcully bcully changed the title Add a hook to filter out documents by query to IndexModule Add a hook to Engine to wrap the DirectoryReader provided to Searchers Oct 2, 2025
Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@bcully bcully merged commit fa1b376 into elastic:main Oct 2, 2025
34 checks passed
@bcully bcully deleted the reshard-search-filter branch October 2, 2025 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue serverless-linked Added by automation, don't add manually Team:Distributed Indexing Meta label for Distributed Indexing team v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants